xenstore,libxl: cleanup of xenstore connections across fork()

Provide a new function xs_daemon_destroy_postfork which can be called
by a libxenstore user who has called fork, to close the fd for the
connection to xenstored and free the memory, without trying to do
anything to any threads which libxenstore may have created.

Use this new function in libxl_fork, to avoid accidental use of a
xenstore connection in both parent and child.

Also, fix the doc comment for libxl_spawn_spawn to have the success
return codes the right way round.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2cc9602d47930c85bf2ed5fcef870b20e20a78ce..f96b3ae035192c3abfbf415f5f90863237edba8a 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -66,7 +66,7 @@ int libxl_ctx_free(struct libxl_ctx *ctx)
     libxl_free_all(ctx);
     free(ctx->alloc_ptrs);
     xc_interface_close(ctx->xch);
-    xs_daemon_close(ctx->xsh); 
+    if (ctx->xsh) xs_daemon_close(ctx->xsh); 
     return 0;
 }
 

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index fea552b70ce1529ab6034f79690a1e15b250fe41..02bdd219439e4e45acad2d34b044471d5f82bdbc 100644 (file)
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -250,6 +250,7 @@ enum {
 int libxl_ctx_init(struct libxl_ctx *ctx, int version);
 int libxl_ctx_free(struct libxl_ctx *ctx);
 int libxl_ctx_set_log(struct libxl_ctx *ctx, libxl_log_callback log_callback, void *log_data);
+int libxl_ctx_postfork(struct libxl_ctx *ctx);
 
 /* domain related functions */
 int libxl_domain_make(struct libxl_ctx *ctx, libxl_domain_create_info *info, uint32_t *domid);

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 10e72e520f2f4156fab51875ee88ae5bea5aed71..29816e7c3696d25436cef29f5b3304684d252bb9 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -185,8 +185,8 @@ int libxl_spawn_spawn(struct libxl_ctx *ctx,
                       void (*intermediate_hook)(void *for_spawn, pid_t innerchild));
   /* Logs errors.  A copy of "what" is taken.  Return values:
    *  < 0   error, for_spawn need not be detached
-   *   +1   caller is now the inner child, should probably call libxl_exec
-   *    0   caller is the parent, must call detach on *for_spawn eventually
+   *   +1   caller is the parent, must call detach on *for_spawn eventually
+   *    0   caller is now the inner child, should probably call libxl_exec
    * Caller, may pass 0 for for_spawn, in which case no need to detach.
    */
 int libxl_spawn_detach(struct libxl_ctx *ctx,

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index c6989cef54bc3079fbb15ced2a33ae2e3428213c..c4dc74c6030925cb053147e5f8b6b01ec58c19d2 100644 (file)
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -282,6 +282,13 @@ READ_WRITE_EXACTLY(read, 1, /* */)
 READ_WRITE_EXACTLY(write, 0, const)
 
 
+int libxl_ctx_postfork(struct libxl_ctx *ctx) {
+    if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
+    ctx->xsh = xs_daemon_open();
+    if (!ctx->xsh) return ERROR_FAIL;
+    return 0;
+}
+
 pid_t libxl_fork(struct libxl_ctx *ctx)
 {
     pid_t pid;
@@ -292,6 +299,14 @@ pid_t libxl_fork(struct libxl_ctx *ctx)
         return -1;
     }
 
+    if (!pid) {
+        if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
+        ctx->xsh = 0;
+        /* This ensures that anyone who forks but doesn't exec,
+         * and doesn't reinitialise the libxl_ctx, is OK.
+         * It also means they can safely call libxl_ctx_free. */
+    }
+
     return pid;
 }
 

diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index 9707d19ca28d244c72a2d22aeca0085af46669f4..5b052684812d0a2cbc4d28374a258ce037a0d78d 100644 (file)
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -223,10 +223,39 @@ struct xs_handle *xs_domain_open(void)
        return get_handle(xs_domain_dev());
 }
 
-void xs_daemon_close(struct xs_handle *h)
-{
+static void close_free_msgs(struct xs_handle *h) {
        struct xs_stored_msg *msg, *tmsg;
 
+       list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
+               free(msg->body);
+               free(msg);
+       }
+
+       list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
+               free(msg->body);
+               free(msg);
+       }
+}
+
+static void close_fds_free(struct xs_handle *h) {
+       if (h->watch_pipe[0] != -1) {
+               close(h->watch_pipe[0]);
+               close(h->watch_pipe[1]);
+       }
+
+        close(h->fd);
+        
+       free(h);
+}
+
+void xs_daemon_destroy_postfork(struct xs_handle *h)
+{
+        close_free_msgs(h);
+        close_fds_free(h);
+}
+
+void xs_daemon_close(struct xs_handle *h)
+{
        mutex_lock(&h->request_mutex);
        mutex_lock(&h->reply_mutex);
        mutex_lock(&h->watch_mutex);
@@ -239,28 +268,13 @@ void xs_daemon_close(struct xs_handle *h)
        }
 #endif
 
-       list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
-               free(msg->body);
-               free(msg);
-       }
-
-       list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
-               free(msg->body);
-               free(msg);
-       }
+        close_free_msgs(h);
 
        mutex_unlock(&h->request_mutex);
        mutex_unlock(&h->reply_mutex);
        mutex_unlock(&h->watch_mutex);
 
-       if (h->watch_pipe[0] != -1) {
-               close(h->watch_pipe[0]);
-               close(h->watch_pipe[1]);
-       }
-
-       close(h->fd);
-
-       free(h);
+        close_fds_free(h);
 }
 
 static bool read_all(int fd, void *data, unsigned int len)

diff --git a/tools/xenstore/xs.h b/tools/xenstore/xs.h
index bd36a0b3502134d56a1c15be0459c70d2440d3d0..4dae5944ae94c787e82caad59cc222c30dd9500e 100644 (file)
--- a/tools/xenstore/xs.h
+++ b/tools/xenstore/xs.h
@@ -48,6 +48,9 @@ struct xs_handle *xs_daemon_open_readonly(void);
 /* Close the connection to the xs daemon. */
 void xs_daemon_close(struct xs_handle *);
 
+/* Throw away the connection to the xs daemon, for use after fork(). */
+void xs_daemon_destroy_postfork(struct xs_handle *);
+
 /* Get contents of a directory.
  * Returns a malloced array: call free() on it after use.
  * Num indicates size.